Repository navigation
🤖 tests: cover sendTreeMessage paths before the split - #7708
Conversation
Characterization tests for the five sendTreeMessage paths no test reached: the relation re-check under the target's lock, the retry tag-generation fence, the admission probe's interrupted-target and missing-target refusals, and the retry whose send fails after a new delegated turn. No production change. Signed-off-by: Thomas Kosiewski <tk@coder.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
@codex review |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
🟢 e2e stories (Haiku): 14 passed22 agent steps · 7 replayed from cache · 15 model calls · 46.5k tokens (24% cached) All 14 tests in 9 files
e2e 0.20.0 · 1m 23s · web · run artifacts 🟢 e2e Electron stories (Haiku): 3 passed3 agent steps · 3 model calls · 7.9k tokens (22% cached) All 3 tests in 3 files
e2e 0.20.0 · 23.7s · electron · run artifacts 🟢 e2e agent repros (Haiku): no tests selectedSpend: $0.00 of the $1.00 cap (proxy ledger) e2e 0.20.0 · 7.1s · 2 targets (web, phone) · run artifacts |
Covers the refusal in the delegated-turn wait: a first peer send that would wait for another workspace's delegated turn is refused, not parked, when the user stopped the target after the send was admitted. The test asserts the refused result, that nothing is parked, sent or counted as delivered. No production change. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$9.55`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high costs=9.55 --> Signed-off-by: Thomas Kosiewski <tk@coder.com>
|
@codex review |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
@codex security review |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
…oder#7709) ## Summary Part 1 of 5: splits the tail of `TaskService.sendTreeMessage` (park, dispatch and settle) in place into named steps, with no behaviour change. The method drops from CC 133 to CC 109. On top of the test PR coder#7708. 460 changed lines. The parts go from the end of the method to its start, as in the two earlier stacks (coder#7697 to coder#7699, coder#7702 to coder#7706). The code that stays inline must be the start of the method, so it can call the extracted tail with one call. ## Steps in this part - `dispatchPeerSend` makes the dispatch. - `refuseStalePeerSendBeforeDispatch` runs the recheck before dispatch. - `queuePeerSendBehindParked` queues the send behind a parked one. - `parkPeerSendForDelegatedTurn` is the former `park` closure, and `waitForDelegatedTurn` waits for the delegated turn. - `settlePeerSendResult` keeps the refusal order: the delegated-turn refusal, then the admission refusal, then `send_failed`. - This part adds the readonly records `PeerSendRequest`, `PeerSendRoute` and `PeerSendAdmission`, and the mutable `PeerSendAttempt`. In this part the admission check is still a closure, and `dispatchPeerSend` gets it as a parameter. ## Rules every part keeps - The four overloads and the implementation signature of `sendTreeMessage` do not change. No step calls `this.sendTreeMessage` again, so the spy in `taskService.delegatedTurnPeerDelivery.test.ts` still sees every call. - Every step runs inside `withPeerAdmissionLock(targetId, ...)`, which is the peer delivery lock plus the target's event lock. No step takes either lock, and the callback returns the chain, so both locks release when the chain settles. - The callbacks that `sendMessage` and the workspace service run later (`admissionStale`, park, `onCanceled`, `onAccepted`) read and write one mutable `PeerSendAttempt` record when they run: `delegatedTurnCorrelation`, `admissionRefusal`, `refusedAtRegistrationEpoch`, `parkedThisAttempt` and `accepted`. `admissionStale` stays a plain synchronous function. - The steps form a chain: each step calls the next one directly after its own last await, so no collaborator call runs a microtask later. Only the result of the send settles later, at approximately 2 to 3 microtasks per chain level. So the two locks also release that much later. A refusal before the checkout await settles at the same time as before. - `make crap-check` against the parent commit reports 0 violations, with and without coverage. Every new function scores CC 8 or less. - A script compared the old and new code: both have 5 awaits and the same collaborator calls. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$198.09`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high costs=198.09 -->
Summary
Adds 6 characterization tests for
TaskService.sendTreeMessage, with no production change. Line coverage of the method goes from 98 % (529 of 538 lines) to 100 %. This is the test PR at the start of stack 6 of the TaskService hotspot split. The split PRs on top of it split the method (CC 133) into named steps.What the tests cover
The new file is
taskService.sendTreeMessage.test.ts. Every test uses two root chats with a shared tag: alpha sends to beta.invalid_scope.invalid_scope.refused, the same result as a send to a target that is already interrupted.not_found.send_failed.refused. Nothing parks, nothing sends, and no delivery is recorded.interruptedParentWorkspaceIdsset directly. The public interrupt also bumps the stop epoch, and the admission check reads the epoch first, so no public call reaches this backstop. A comment in the test says so.sendMessage, then run the same admission check the host runs. The checkout await has no hook that a test can use.Validation
if. It passes on the unsplit code and on the split code, and it fails on both when the branch is removed.Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:high• Cost:$198.09